ZeroGC: stop depending on MethodTable internal layout for IsLargeObject - #3298
Merged
kkokosa merged 4 commits intoAug 4, 2026
Merged
Conversation
added 2 commits
August 3, 2026 16:28
gcperfsim-cache (Workstation and Server GC modes) previously had no raw per-second dotnet-counters CSV capture in results/raw/, only the ZeroGC capture existed. This meant its TimeSeries.PauseTimePct chart data (stride- decimated to ~150 points from the full capture) could not be cross-checked or re-derived from source, and its reported PauseTimePctStats.Max included a rare one-off pause spike (11.80% Workstation) that the decimated chart series did not show (chart max ~0.46%), a ~26x visual understatement in report.html. This change re-runs gcperfsim-cache for both Workstation and Server GC for the same 600s duration as the rest of the suite, using an isolated output directory so the other 31 existing runs in results-full.json are left untouched, then merges just the two refreshed gcperfsim-cache entries back in and regenerates report.html (synced to docs/zerogc/report.html). Note: the new capture's Workstation run does not reproduce the prior rare pause spike (new Max 0.47% vs old 11.80%) -- GC pause spikes are inherently stochastic (e.g. one-off blocking Gen2 collection under memory pressure), so this is expected run-to-run noise, not a regression. Avg/P50/P90 for Workstation are consistent with the prior run (Avg 0.142% -> 0.120%, P50 0.149% -> 0.147%, P90 0.354% -> 0.350%). Full raw CSVs are now checked in for both modes, closing the previously-missing data gap.
…ject IsLargeObject(Object*) was the only place in ZeroGC that dereferenced a MethodTable's internal fields (via GetGCSafeMethodTable()->GetBaseSize()). MethodTable/Object's field layout (as mirrored in gcenv.object.h) is private VM implementation detail, not part of the versioned GC/EE interface (gcinterface.h's GC_INTERFACE_MAJOR/MINOR_VERSION) - it can change shape across runtime major versions with no interface version bump at all, which is exactly why ZeroGC.dll binaries are pinned to one target runtime major version. Since ZeroGC already segregates large/pinned objects into their own dedicated arena chunk at allocation time (the isLarge branch in AllocateFromArena), it already knows which objects are large without asking the object itself. This change records the exact address range of every allocation that meets the LOH size threshold (alignedSize >= LARGE_OBJECT_SIZE, matching the removed check's semantics precisely - not the broader isLarge flag, which also covers small pinned/POH allocations) into a small std::map<start, end> under a critical section, and answers IsLargeObject via a range lookup instead. This removes ZeroGC's only remaining runtime-internal-layout dependency for object introspection. The gcenv.*.h shim headers are still required (their Object*/MethodTable*/gc_alloc_context* type names appear throughout the mandatory IGCHeap/IGCHandleManager interface signatures - unavoidable for any standalone GC), but ZeroGC no longer dereferences any field inside MethodTable's actual struct layout anywhere, so that header's internal field-layout stability no longer matters for this GC's correctness. Verified: builds clean (native/build.ps1), and smoke-tested against GCPerfSim with -lohar/-lohsr/pinned-object args (exercises the LOH and POH allocation paths this change touches) - no crashes, collection_counts stay [0,0,0] as expected for ZeroGC.
Member
This is invalid statement. MethodTable data required to walk the object graph are a binary contract between the GC and EE for performance reasons. Regular shipping GC depends on this data heavily. Changing this data in incompatible way would definitely require major GC/EE interface version bump. |
The IsLargeObject fix in the previous commit (removing the MethodTable
dependency) was only smoke-tested by running GCPerfSim/ConsoleApp against
a Release build - which never actually exercises IsLargeObject at all.
Its only two real call sites in dotnet/runtime are both unreachable in a
normal Release build:
- Object::ValidateInner (vm/object.cpp) requires USE_CHECKED_OBJECTREFS,
which is only defined under _DEBUG (src/coreclr/inc/switches.h:20).
- Object::ValidateHeap (gc/gcinternal.h) requires the VERIFY_HEAP
compile-time define, only set for the reference GC's own CMake build
(gc/CMakeLists.txt), not for an EE/host consuming a standalone GC.
This adds a standalone native unit test (test_islargeobject.cpp) that
links directly against ZeroGCHeap.cpp/ZeroGCHandles.cpp/dllmain.cpp and
calls Alloc()/IsLargeObject() directly, bypassing the EE entirely, giving
real, deterministic coverage of the map-based large-object-range tracking:
- small vs. just-under-threshold vs. at/above-threshold objects
- GC_ALLOC_LARGE_OBJECT_HEAP / GC_ALLOC_PINNED_OBJECT_HEAP flagged
allocations below the byte threshold (must NOT be 'large')
- unrelated (non-heap) pointers
- back-to-back and interleaved small/large allocation sequences
All 16 assertions pass. Added run-tests.ps1 to build and run it.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This reverts commit f1fbfd7.
Member
Author
I agree but this is by convention, not by structured versioning. |
Member
The GC interface version covers both the methods on the C++ interfaces, and behaviors and data contracts that do not show up as methods on the C++ interfaces. For example, dotnet/runtime#91821 changed the MethodTable data and bumped the This change is unnecessary overengineering. |
Member
Author
|
In that sense, I agree and it adds unnecessary complexity - reverted via #3299, thanks |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Status: reverted, keeping open for the record
This change has been reverted by a follow-up commit on this same branch (d21e00d30d0), restoring the original
pObj->GetGCSafeMethodTable()->GetBaseSize() >= LARGE_OBJECT_SIZEimplementation ofIsLargeObject.Per review discussion: the GC/EE data-layout contract is versioned in a structured way, not just by convention. The one concrete historical layout break this PR was defending against (
MethodTable::Collectible()'s flag bit remap, dotnet/runtime#91821) bumpedEE_INTERFACE_MAJOR_VERSION1→2, andgcenv.object.h's existing shim (g_oldMethodTableFlags) - which ZeroGC'sdllmain.cppalready plugs into - branches on exactly that signal.MethodTable::GetBaseSize()itself has never needed such a shim. Since ZeroGC can't avoid depending onMethodTable/Objectlayout elsewhere anyway (mandatory for theIGCHeapinterface signatures), removing this one call site added real ongoing cost (astd::map+ lock on every LOH-sized allocation) without closing off an actual risk. Original description/rationale kept below for context.Summary (original, superseded)
Removes ZeroGC's only dependency on MethodTable's internal field layout, in response to a question about whether ZeroGC breaks the standalone-GC design goal (per Maoni) of letting a newer GC binary run against an older runtime as long as possible.
Background
CoreCLR's actual GC/EE interface (
gcinterface.h'sGC_INTERFACE_MAJOR_VERSION/GC_INTERFACE_MINOR_VERSION) is explicitly designed for this: the EE only rejects a loaded GC whose reported major version is older than expected (gcheaputilities.cpp) - an equal-or-newer GC is accepted. That mechanism is untouched by ZeroGC and still works as designed.Where ZeroGC actually diverges: it's compiled directly against the VM's internal object-model headers (
gcenv.object.h'sMethodTable/Objectclasses, which mirror the real VM-internal layout byte-for-byte) rather than staying entirely within the versioned public interface surface. Auditing the codebase, this was needed in exactly one place:ZeroGCHeap::IsLargeObject, which calledpObj->GetGCSafeMethodTable()->GetBaseSize().What changed (now reverted)
IsLargeObjectno longer touched the object'sMethodTableat all - it recorded the address range of every LOH-sized allocation into astd::map<start, end>guarded by a critical section, and looked pointers up against that map instead.Why reverted
See "Status" above - the risk this defended against is already covered by the existing
EE_INTERFACE_MAJOR_VERSION-gated shim mechanism ingcenv.object.h, so the extra bookkeeping/locking on the hot allocation path wasn't buying real safety.